Skip to content

perf(iterator): elide y.ParseKey in hasPrefix fast path#2293

Open
shaunpatterson wants to merge 2 commits into
dgraph-io:mainfrom
shaunpatterson:perf/iterator-hasprefix-fast-path
Open

perf(iterator): elide y.ParseKey in hasPrefix fast path#2293
shaunpatterson wants to merge 2 commits into
dgraph-io:mainfrom
shaunpatterson:perf/iterator-hasprefix-fast-path

Conversation

@shaunpatterson
Copy link
Copy Markdown

Summary

hasPrefix runs on every iterator.Next() call. Previously it always called y.ParseKey(iitr.Key()) (nil check + sub + slice) before delegating to bytes.HasPrefix. When the user-supplied Prefix fits entirely within the userKey portion (len(internalKey) >= len(Prefix)+8), bytes.HasPrefix on the full internal key is equivalent to bytes.HasPrefix on ParseKey(internalKey) — the prefix bytes can only land inside the userKey region, never inside the 8-byte ts suffix.

The fallback (prefix longer than userKey) still takes the ParseKey path so we don't spuriously match against ts bytes.

Motivation

dgraph's posting/mvcc.go ReadPostingList walks all versions of a single key on every cache miss and every opRollup batch (16 keys per batch, continuous). hasPrefix is on that hot loop.

Tests added

  • TestRegressionHasPrefixFastPath: prefix < userKey hits the fast path
  • TestRegressionHasPrefixShortPrefixFallback: prefix > userKey hits the fallback and correctly returns false

Test plan

  • go test ./... passes
  • New regression tests verify both the fast path and the ParseKey fallback

🤖 Generated with Claude Code

`hasPrefix` runs on every iterator.Next() call. Previously it always
called `y.ParseKey(iitr.Key())` (nil check + sub + slice) before
delegating to bytes.HasPrefix. When the user-supplied Prefix fits
entirely within the userKey portion (len(internalKey) >= len(Prefix)+8),
bytes.HasPrefix on the full internal key is equivalent to bytes.HasPrefix
on ParseKey(internalKey) — the prefix bytes can only land inside the
userKey region, never inside the 8-byte ts suffix. The fallback (prefix
longer than userKey) still takes the ParseKey path so we don't spuriously
match against ts bytes.

Direct motivation: dgraph's posting/mvcc.go ReadPostingList walks all
versions of a single key on every cache miss and every opRollup batch
(16 keys per batch, continuous). hasPrefix is on that hot loop.

Tests added:
- TestRegressionHasPrefixFastPath: prefix < userKey hits the fast path
- TestRegressionHasPrefixShortPrefixFallback: prefix > userKey hits the
  fallback and correctly returns false

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@shaunpatterson shaunpatterson requested a review from a team as a code owner May 26, 2026 00:44
The previous fixture used userKey="a" with prefix="abcdefghij". Seek lands
past EOF (since "a" sorts before "abcdefghij") so hasPrefix is never
called and the ParseKey fallback branch stays uncovered.

Switch to userKey="z" with prefix="yy" — "z" sorts after "yy" so Seek
lands on it, hasPrefix() runs, and the len(key)=9 < len(prefix)+8=10
test forces the fallback path. Confirmed: line 736 now hit in coverage.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant